Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translate more OpenTelemetry resource conventions #4955

Merged
merged 4 commits into from
Mar 18, 2021

Conversation

axw
Copy link
Member

@axw axw commented Mar 15, 2021

Motivation/summary

Translate more OpenTelemetry resource conventions, focusing on common conventions which already have a clear mapping to our data model. Add fields to metadata model types for missing ECS fields.

Checklist

How to test these changes

  1. Run an OpenTelemetry-instrumented application with rich instrumentation (e.g. Java auto-instrumentation), pointed at apm-server
  2. Wait for events to be recorded, check:
    • Labels for untranslated attributes
    • Captured field values are sensible

Related issues

#4714

Translate more OpenTelemetry resource conventions,
focusing on common conventions ones which already
have a clear mapping to our data model. Add fields
to metadata model types for missing ECS fields.
@axw axw force-pushed the otel-resource-conventions branch from cf62e63 to bdaebf3 Compare March 15, 2021 06:44
@apmmachine
Copy link
Contributor

apmmachine commented Mar 15, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #4955 updated

  • Start Time: 2021-03-18T02:20:49.229+0000

  • Duration: 42 min 41 sec

  • Commit: 3229b8a

Test stats 🧪

Test Results
Failed 0
Passed 4791
Skipped 117
Total 4908

Trends 🧪

Image of Build Times

Image of Tests

Update modeldecodertest.SetStructValues with options
which enable a caller to override the value to which
a field is set. This is used to initialise fields
which have no equivalent in the input model.
@axw axw marked this pull request as ready for review March 15, 2021 09:25
@axw axw requested a review from a team March 15, 2021 09:25
Comment on lines -38 to -49
"hostname": System{DetectedHostname: detected},
"ignored hostname": System{ConfiguredHostname: configured},
"full hostname info": System{DetectedHostname: detected, ConfiguredHostname: configured},
"k8s nodename with hostname": System{Kubernetes: Kubernetes{NodeName: nodename}, DetectedHostname: detected},
"k8s nodename with configured hostname": System{Kubernetes: Kubernetes{NodeName: nodename}, ConfiguredHostname: configured},
"k8s podname": System{Kubernetes: Kubernetes{PodName: podname}, DetectedHostname: detected},
"k8s podUID": System{Kubernetes: Kubernetes{PodUID: podUID}, DetectedHostname: detected},
"k8s namespace": System{Kubernetes: Kubernetes{Namespace: namespace}, DetectedHostname: detected},
"k8s podname with configured hostname": System{Kubernetes: Kubernetes{PodName: podname}, DetectedHostname: detected, ConfiguredHostname: configured},
"k8s podUID with configured hostname": System{Kubernetes: Kubernetes{PodUID: podUID}, DetectedHostname: detected, ConfiguredHostname: configured},
"k8s namespace with configured hostname": System{Kubernetes: Kubernetes{Namespace: namespace}, DetectedHostname: detected, ConfiguredHostname: configured},
"k8s empty": System{Kubernetes: Kubernetes{}, DetectedHostname: detected, ConfiguredHostname: configured},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubernetes is generally covered by kubernetes_test.go, and the logic for setting host.hostname for events with kubernetes metadata is covered by the modeprocessor. The tests are redundant now.

Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@codecov-io
Copy link

Codecov Report

Merging #4955 (c6ffd93) into master (9ab53ab) will increase coverage by 0.22%.
The diff coverage is 93.44%.

@@            Coverage Diff             @@
##           master    #4955      +/-   ##
==========================================
+ Coverage   76.98%   77.21%   +0.22%     
==========================================
  Files         175      175              
  Lines       10354    10413      +59     
==========================================
+ Hits         7971     8040      +69     
+ Misses       2383     2373      -10     
Impacted Files Coverage Δ
processor/otel/metadata.go 88.69% <91.48%> (+22.51%) ⬆️
model/container.go 100.00% <100.00%> (ø)
model/process.go 100.00% <100.00%> (ø)
model/system.go 86.66% <100.00%> (+4.84%) ⬆️
...ack/apm-server/aggregation/txmetrics/aggregator.go 93.24% <0.00%> (ø)

@axw axw merged commit 1b1e79d into elastic:master Mar 18, 2021
@axw axw deleted the otel-resource-conventions branch March 18, 2021 03:08
axw added a commit to axw/apm-server that referenced this pull request Mar 29, 2021
* Translate more OpenTelemetry resource conventions

Translate more OpenTelemetry resource conventions,
focusing on common conventions ones which already
have a clear mapping to our data model. Add fields
to metadata model types for missing ECS fields.

* model/modeldecoder: fix tests due to added fields

Update modeldecodertest.SetStructValues with options
which enable a caller to override the value to which
a field is set. This is used to initialise fields
which have no equivalent in the input model.
# Conflicts:
#	changelogs/head.asciidoc
axw added a commit that referenced this pull request Mar 29, 2021
* Translate more OpenTelemetry resource conventions

Translate more OpenTelemetry resource conventions,
focusing on common conventions ones which already
have a clear mapping to our data model. Add fields
to metadata model types for missing ECS fields.

* model/modeldecoder: fix tests due to added fields

Update modeldecodertest.SetStructValues with options
which enable a caller to override the value to which
a field is set. This is used to initialise fields
which have no equivalent in the input model.
# Conflicts:
#	changelogs/head.asciidoc
@simitt
Copy link
Contributor

simitt commented May 12, 2021

Tested with BC 4 - looks good.

Different labels were indexed:
Screenshot 2021-05-12 at 15 31 09
Screenshot 2021-05-12 at 15 31 47
Screenshot 2021-05-12 at 15 32 04

@simitt simitt self-assigned this May 12, 2021
mergify bot pushed a commit that referenced this pull request May 12, 2021
* Translate more OpenTelemetry resource conventions

Translate more OpenTelemetry resource conventions,
focusing on common conventions ones which already
have a clear mapping to our data model. Add fields
to metadata model types for missing ECS fields.

* model/modeldecoder: fix tests due to added fields

Update modeldecodertest.SetStructValues with options
which enable a caller to override the value to which
a field is set. This is used to initialise fields
which have no equivalent in the input model.

(cherry picked from commit 1b1e79d)

# Conflicts:
#	changelogs/head.asciidoc
#	model/modeldecoder/v2/metadata_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants